-
Notifications
You must be signed in to change notification settings - Fork 9
chore: SSE bug fixes #639
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: SSE bug fixes #639
Conversation
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request. |
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/in-app-sse #639 +/- ##
========================================================
+ Coverage 67.85% 68.30% +0.44%
- Complexity 748 760 +12
========================================================
Files 142 142
Lines 4303 4303
Branches 580 580
========================================================
+ Hits 2920 2939 +19
+ Misses 1165 1141 -24
- Partials 218 223 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
|
Build available to test |
📏 SDK Binary Size Comparison Report
|
messaginginapp/src/main/java/io/customer/messaginginapp/gist/presentation/GistModalActivity.kt
Outdated
Show resolved
Hide resolved
|
|
mrehan27
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. Just two questions before we merge this:
- Do we also need to add message equality check in subscriber (
subscribeToAttributes), or is adding it inonDestroysufficient? - Can we add some tests to cover these changes?
messaginginapp/src/main/java/io/customer/messaginginapp/gist/presentation/GistModalActivity.kt
Outdated
Show resolved
Hide resolved
7047457 to
7726fa2
Compare
|
|
7726fa2 to
e795523
Compare
|
|
|
|
The issue
This PR fixes an issue for in-app Android. Sometimes when there are multiple messages the
GistModalActivityanimation takes longer than the processor to update the state. In which case, the activity dismisses the wrong message.onDestroyof old activity runs and grabs the current message -> which is now message 2Dismisses it as per this
This is not really related to SSE but it's more apparent because SSE shows subsequent messages faster.
The fix
GistModalActivityis created, it holds on to the message it was created to showonDestroyruns, we make sure not to call dismiss unless our message is being shownNote
Fix race condition in modal dismissal
GistModalActivity: trackactivityMessageand inonDestroydispatchDismissMessageonly if itsqueueIdmatches the currently displayed message; persistent messages dismiss withshouldLog = false. Minor refactor:isPersistentMessage(message: Message?).InAppMessagingMiddlewares: after dismissal, gate queue processing bystore.state.shouldUseSse(wassseEnabled).InAppMessagingManager.Written by Cursor Bugbot for commit e795523. This will update automatically on new commits. Configure here.